-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding check_parameter_positivity()
function to seir.py
#428
Conversation
Still need to add the test function (not ready yet for review) |
@jcblemai I have initial concerns that I did not properly instantiate |
I'm very, very bad at naming things (Carl, Tim are definitely very good at this) but neg_param() is not informative enough as a function name, something like |
parsed_parameter has shape (n_parsed_parameters(unique_strings) X n_times X n_subpop), is that what's causing the CI error ? EDIT: no, see below: |
neg_params()
function to seir.py
check_parameter_positivity()
function to seir.py
It turns out there's only 30 days in April...
Small documentation changes and removing an unrelated but unnecessary loc from `test_seir.py`
Incorporating the information within a `print()` statement into the `ValueError` output in `seir.py::check_parameter_positivity()`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation is great, thanks for doing that! The testing looks good too, I just have some brief comments on that front, should be quick to fix.
My biggest concern is that the error message will be too verbose, I think we want to convey to users quickly the most important thing. Multi-line error messages confuse users and can make it more difficult to diagnose.
oops forgot to do this before previous push
…in `test_seir.py`
@jcblemai Finally fixed the errors with this and it is ready to be merged, but I wanted to get your approval. Everything look good/does it do what you want? |
Describe your changes.
This pull request introduces a function called
check_parameter_positivity()
toseir.py
.check_parameter_positivity()
takes in an array of parsed parameters, as well as parameter names, subpopulation names, and dates, checks for the existence of negative parameter values, and throws aValueError
error if they are found. When throwing the error,check_parameter_positivity()
will only print the earliest (w.r.t date) negative parameter to avoid redundant feedback. Example output will resemble:A test function,
test_check_parameter_positivity()
has also been added totest_seir.py
to testcheck_parameter_positivity()
.Does this pull request make any user interface changes? If so please describe.
No changes to the user interface.
What does your pull request address? Tag relevant issues.
This pull request addresses GH #215.
Tag relevant team members.
@jcblemai